feat: add ServiceAccountImagePullProfile for identity-binding-based image pull #7596
Conversation
qweeah
changed the title
qweeah
requested review from
AbelHu,
Devinwong,
awesomenix,
cameronmeissner,
djsly,
ganeshkumarashok,
juan-lee,
junjiezhang1997,
lilypan26,
phealy,
r2k1,
timmy-wright and
zachary-bailey
as code owners
|
This PR's logic LGTM. But I don't quiet understand what is step 2 "RP integration and configuration flow". Agentbaker won't rely on anything from AKS RP, I think you could make all changes in the same PR and add E2E for it? Here is an example PR: #7059 . |
|
The latest Buf updates on your PR. Results from workflow Buf CI / buf (pull_request).
|
There was a problem hiding this comment.
Pull request overview
This pull request adds support for service-account-based image pull authentication (Identity Binding) from Azure Container Registry. The feature implements KEP-4412 projected service account tokens for kubelet image credential providers.
Changes:
- Added
ServiceAccountImagePullProfiledata structure with fields for enabled status, default client/tenant IDs, and Local Authority SNI - Extended both legacy CSE (template-based) and modern AKSNodeConfig (proto-based) paths to support the new configuration
- Modified credential provider configuration generation to include identity binding token attributes and CLI arguments when enabled
Reviewed changes
Copilot reviewed 44 out of 131 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
pkg/agent/datamodel/types.go |
Added ServiceAccountImagePullProfile struct to Properties |
pkg/agent/variables.go |
Added getter functions for service account image pull configuration with null-safe access |
pkg/agent/variables_test.go |
Added unit tests for new variables in Windows CSE and custom data |
parts/linux/cloud-init/artifacts/cse_config.sh |
Refactored credential provider config generation to support identity binding parameters |
spec/parts/linux/cloud-init/artifacts/cse_config_spec.sh |
Added comprehensive ShellSpec tests for all credential provider scenarios |
pkg/agent/testdata/*/CustomData |
Updated snapshot test data with new environment variables |
pkg/agent/testdata/*/CSECommand |
Updated snapshot test data with new environment variables |
Signed-off-by: Billy Zha <[email protected]>
Signed-off-by: Billy Zha <[email protected]>
|
Pending the e2e tests. Probably rebase/merge with main branch will resolve it |
Already rebased to the latest main. Added E2Es are passing. |
What type of PR is this?
/kind feature
What this PR does / why we need it:
This PR adds support for service-account-based image pull authentication (aka Identity Binding) from Azure Container Registry (ACR), implementing KEP-4412 projected service account tokens for kubelet image credential providers.
Changes
Data Model
Added
ServiceAccountImagePullProfiletoSecurityProfilewith fields:Enabled: Enable/disable service account-based image pullDefaultClientID: Default managed identity client IDDefaultTenantID: Default managed identity tenant IDLocalAuthoritySNI: SNI endpoint for Identity Bindings Local AuthorityAdded getter methods to
SecurityProfilefor null-safe access.Implementation Paths
1. Legacy CSE (Template-based)
pkg/agent/variables.go: Template variables for CSE scripts (using the naming conventionserviceAccountImagePull...)parts/linux/cloud-init/artifacts/cse_cmd.sh: Environment variable declarations (e.g.,SERVICE_ACCOUNT_IMAGE_PULL_ENABLED,SERVICE_ACCOUNT_IMAGE_PULL_DEFAULT_CLIENT_ID)parts/linux/cloud-init/artifacts/cse_config.sh: Credential provider config generation2. AKSNodeConfig (Proto-based)
aks-node-controller/proto/aksnodeconfig/v1/security_profile.proto: Proto definitions (ServiceAccountImagePullProfile)aks-node-controller/parser/parser.go: Environment variable generationaks-node-controller/parser/helper.go: Null-safe helper functionsBoth paths converge at
cse_config.shto generate/var/lib/kubelet/credential-provider-config.yamlwith identity binding arguments (--ib-default-client-id,--ib-default-tenant-id,--ib-sni-name) and token attributes.Testing
variables_test.go,datamodel/types_test.go, andparser/helper_test.goto reflect the new naming convention.All tests validate that the credential provider config file contains the correct identity binding configuration.
Cluster Support
Which issue(s) this PR fixes:
Fixes #
Requirements:
Special notes for your reviewer:
Dual implementation approach (legacy CSE + modern AKSNodeConfig) for backward compatibility. Both paths are fully tested and generate identical credential provider configuration. The renaming from
ImagePullIdentitytoServiceAccountImagePullensures consistency with the upstream feature name.Release note: